Add an instant search (via AJAX) #173

voronkovich · 2015-09-04T14:46:49Z

It's almost like the Google... but better

bocharsky-bw · 2015-09-04T14:55:25Z

Wow, it's cool! But I think it's a bit overhead for demo project.

javiereguiluz · 2015-09-04T15:02:32Z

@voronkovich 👍 I like this idea a lot. Thanks for working on it.

However, after a first quick review of the code, I find the TermSplitter and the TermEmphasizer features a bit overkill for the demo project. Please don't make any changes yet, but I think we should get rid of both of them and focus just on the Ajax feature.

voronkovich · 2015-09-04T16:52:04Z

@javiereguiluz, if we remove the TermSplitter we will have to validate the ajax request anyway. The TermSplitter just only remove invalid characters from the query string and splits it into words.
I agree with removing TermEmphasizer. It is overhead, you're right.

voronkovich · 2015-09-05T19:20:47Z

I've refactored the code. All overheads were beheaded.

theofidry · 2015-09-05T20:37:34Z

src/AppBundle/Controller/BlogController.php

let's avoid a 1 char variable name :)

It's vastly using name for a search request. The Symfony search uses this name http://api.symfony.com/2.7/index.html?q=request

Well, variable's name is matching with input name, so I prefer to use $q to be consistent.

yeah, as an URL param, not a PHP variable

voronkovich · 2015-09-06T14:09:47Z

I've renamed the $q variable to $query and removed the $em, because it's not needed here

stof · 2015-09-07T10:53:55Z

src/AppBundle/Controller/BlogController.php

this should be an array. Doctrine does not return collection objects when doing queries

voronkovich · 2015-09-07T11:32:03Z

Fixed. Thanks @stof!

javiereguiluz · 2015-09-09T07:47:42Z

app/Resources/assets/js/jquery.instantSearch.js

do we need a semicolon here to avoid issues in some browsers?

there is no issue here. Browsers are consistent in the way they insert semi-colons

OK. Then I ask for this change because of our own code consistency. Either add this semicolon ... or remove all the other semicolons. Thanks!

stof · 2015-09-09T09:08:12Z

app/Resources/assets/js/jquery.instantSearch.js

your throttling function is flawed: it can be used only once as all usages of the throttle function are sharing the same timer

voronkovich · 2015-09-09T19:27:19Z

Fixed again. Thanks, @stof!

xabbuh · 2015-09-11T19:42:10Z

@voronkovich Seems like this needs a rebase.

voronkovich · 2015-09-11T19:49:57Z

@xabbuh can you shortly explain what exactly I have to do with it?

stof · 2015-09-11T20:46:12Z

See http://symfony.com/doc/current/contributing/code/patches.html#rework-your-patch

stof · 2015-09-18T08:08:44Z

-1 on this implementation for now. We should not advocate strip_tags as a way to produce safe HTML strings, as it is not its behavior (and even less its goal)

javiereguiluz · 2015-09-18T08:27:56Z

OK then. Let's wait a bit to solve the XSS issues :)

voronkovich · 2015-09-18T15:24:39Z

Fixed! Thanks, @stof!

voronkovich · 2015-12-16T14:35:51Z

@javiereguiluz, what about to merge this PR?

@voronkovich

This PR was merged into the master branch. Discussion ---------- Add an instant post search I've taken the code commitedd by @voronkovich in #173 and made some changes to it. Let's see if you like it. I propose to implement it like Google search results instead of displaying it in the main menu: ![search-results](https://user-images.githubusercontent.com/73419/28628630-783fd8e4-7225-11e7-89f3-c5941e92a4df.gif) The reason is that the main menu is crowded when the user is logged in and the search bar looks bad. Commits ------- fe2e2ab Fixed CS issues 51ab96a Added missing search.js file f219db9 Changes requested by reviewers 0c236ed Refactored the feature da43022 Fix cs issues f45c31c Add an instant post search

Koc · 2017-07-27T07:44:04Z

src/AppBundle/Controller/BlogController.php

+        $results = [];
+
+        foreach ($posts as $post) {
+            array_push($results, [


why not just $results[] = xxx ?

@Koc, I don't know :) Anyway, this code was changed in #613.

We finally used $results[] = xxx ... mostly because PHPStorm displayed a pretty annoying message suggesting that 😄

voronkovich force-pushed the instant-search branch 2 times, most recently from 7865f32 to 0f183f4 Compare September 5, 2015 20:05

theofidry reviewed Sep 5, 2015
View reviewed changes

voronkovich force-pushed the instant-search branch from 0f183f4 to c3da6ee Compare September 5, 2015 22:01

stof reviewed Sep 7, 2015
View reviewed changes

javiereguiluz added the Feature label Sep 9, 2015

javiereguiluz reviewed Sep 9, 2015
View reviewed changes

javiereguiluz added the FINISHED label Sep 9, 2015

stof reviewed Sep 9, 2015
View reviewed changes

voronkovich force-pushed the instant-search branch from 25d4be6 to c4b943a Compare September 11, 2015 07:12

voronkovich force-pushed the instant-search branch 3 times, most recently from 7a32167 to a6a3192 Compare September 18, 2015 15:21

voronkovich force-pushed the instant-search branch from a6a3192 to c130256 Compare September 27, 2015 16:28

voronkovich force-pushed the instant-search branch 3 times, most recently from f7098dc to c651e44 Compare October 31, 2015 18:32

voronkovich force-pushed the instant-search branch from c651e44 to f017bb2 Compare December 11, 2015 15:15

javiereguiluz mentioned this pull request Dec 28, 2015

Add title filter in admin using LexikFormFilterBundle #280

Closed

voronkovich force-pushed the instant-search branch 3 times, most recently from 6f700c1 to d187dcd Compare December 28, 2015 22:23

voronkovich force-pushed the instant-search branch 5 times, most recently from 176b753 to 3194067 Compare March 1, 2017 17:06

Add an instant post search

f45c31c

voronkovich force-pushed the instant-search branch from 3194067 to f45c31c Compare July 14, 2017 18:40

Fix cs issues

da43022

javiereguiluz mentioned this pull request Jul 26, 2017

Add an instant post search #613

Merged

javiereguiluz merged commit da43022 into symfony:master Jul 27, 2017

javiereguiluz mentioned this pull request Jul 27, 2017

Add an example using an ajax request #69

Closed

Koc reviewed Jul 27, 2017

View reviewed changes

voronkovich deleted the instant-search branch July 27, 2017 08:17

Uh oh!

Add an instant search (via AJAX) #173

Add an instant search (via AJAX) #173

Uh oh!

Conversation

voronkovich commented Sep 4, 2015

Uh oh!

bocharsky-bw commented Sep 4, 2015

Uh oh!

javiereguiluz commented Sep 4, 2015

Uh oh!

voronkovich commented Sep 4, 2015

Uh oh!

voronkovich commented Sep 5, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

voronkovich commented Sep 6, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

voronkovich commented Sep 7, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

voronkovich commented Sep 9, 2015

Uh oh!

xabbuh commented Sep 11, 2015

Uh oh!

voronkovich commented Sep 11, 2015

Uh oh!

stof commented Sep 11, 2015

Uh oh!

stof commented Sep 18, 2015

Uh oh!

javiereguiluz commented Sep 18, 2015

Uh oh!

voronkovich commented Sep 18, 2015

Uh oh!

voronkovich commented Dec 16, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants